Skip to content

feat(spec)!: FlowSchema refuses a flow whose edges[] declares the same id twice (#14964) - #15716

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-14964-flow-edge-id-uniqueness
Sep 5, 2026
Merged

feat(spec)!: FlowSchema refuses a flow whose edges[] declares the same id twice (#14964)#15716
os-project-manager merged 3 commits into
mainfrom
claude/issue-14964-flow-edge-id-uniqueness

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #14964

Maintainer ruling A (recorded in issue comment 5548575136, decision batch #40, verbatim 「同意」): FlowSchema gains a superRefine on edges[] that refuses a duplicate edge id at parse time, error level, with a message naming the id and both positions. Not taken: B (warning with a dated escalation), C (leave it). Clause-②: yes — a published schema's accept set narrows, so needs:contract-review rides on this PR and on the card.

What changes

  • packages/spec/src/automation/flow.zod.tsFlowSchema is now strictObject(…).superRefine(…). Each later occurrence of an already-declared edge id raises one custom issue at path ['edges', j, 'id'] (the LATER edge), naming both positions. Zod 4 .superRefine returns the same ZodObject, so Flow, FlowParsed, defineFlow, stack.zod.ts and every other importer see an unchanged type; edges[].id keeps its name, type and describe. No warning level, no opt-out, no transition window; nodes[].id is deliberately not covered (see the out-of-scope note).
  • packages/spec/src/automation/flow.test.ts — six pins: the duplicate refused with code: 'custom' at ['edges', 1, 'id'] and the message naming edges[1] / edges[0]; the rendered formatZodError line; one issue per later occurrence, each naming the FIRST declaration (5-edge interleaved fixture); the card's control (invalid edge type) still refused with invalid_value at ['edges', 1, 'type'] and no duplicate issue beside it; a unique-id flow of the same shape accepted with ids in authored order via both safeParse and defineFlow; defineFlow refusing the duplicate with the same anchored issue.
  • .changeset/flow-edge-id-uniqueness.md@objectstack/spec minor with the BREAKING banner and the ADR-0087 disposition not-required (no-migration-prescription) (nothing renamed or retired; the remedy is renumbering, and the census names no in-repo file). No migration registry entry is written (packages/spec/src/migrations/registry.ts untouched).

Generated followers: none moved — check:generated reports all 15 artifacts up to date (authorable-surface/automation.json, content/docs/references/automation/flow.mdx, strictness ledger and liveness counts unchanged), as expected for a refinement that adds no key.

Rendered refusal

Validation failed (1 issue):

  ✗ edges.1.id: Duplicate edge id `dup` — `edges[1]` reuses the id already declared by `edges[0]`; every edge id in a flow must be unique. Renumber one of them: an edge id is the handle a designer, a BPMN export or a flow diff keys on, so a collision is silently wrong there rather than loudly broken.

Probe reproduced on today's origin/main before editing

Tree a55efc6c1 (clean), FlowSchema imported from source via tsx, the card's three-part probe:

PROBE duplicate ids   → success: true   parsed edge ids: ["dup","dup"]
CONTROL invalid type  → success: false  ✗ edges.1.type: Invalid option: expected one of "default"|"fault"|"conditional"|"back"
UNIQUE ids            → success: true

Premise valid: the accept was a missing rule on a live validator, exactly as the card measured on 17.2.0.

Zone-2 census — duplicate edge ids, with a lit control

Method. An AST scanner (TypeScript compiler API for .ts/.tsx/.mts/.cts/.js/.mjs/.cjs/.jsx; a structural walk for .yaml/.yml/.json) over every edges: [ … ] array literal — property assignments, edges variable declarations, arrays inside defineFlow(…) calls, as const / parenthesised wrappers unwrapped — collecting each element's literal id (string or no-substitution template) and COUNTING every element it cannot decide (computed id, spread, non-object element) rather than dropping it. Roots: packages/**, examples/** (the ruling's qa/** does not exist at the repo root; packages/qa/** is inside packages/**); node_modules, dist, .turbo, .cache, coverage skipped.

Control (lit). A synthetic root with three TS duplicate shapes (a property assignment, an edges: inside defineFlow(…) with a template-literal id and as const, a variable-declared edges array), one YAML and one JSON duplicate, plus a negative file with a spread and a computed id: the scanner reports exactly the five planted duplicate arrays with their positions and zero for the negative file (its spread and computed id counted as undecided).

Static reading, objectstack tree a55efc6c1: 5,703 code + 366 data files scanned · 776 edges[] arrays · 1,098 edges · 1,052 literal ids · 8 computed ids · 8 spreads · 30 non-object elements · 24 arrays with at least one undecidable element (all of them test fixtures under packages/lint, packages/services/service-automation, packages/spec and the BPMN mapper's computed spreads in bpmn-mapping.ts — none an authored flow under examples/**) · 0 arrays with a duplicate literal id.

Dynamic reading (the NEW refinement over each parsed flow): every flow file the example apps ship (examples/app-crm/src/flows, examples/app-showcase/src/automation/flows, examples/app-todo/src/flows — 7 files, 73 flow-shaped exports counting re-exports) imported and parsed through the built FlowSchema from dist/automation: 0 refused; the control (a synthetic two-edge duplicate through the same path) refused with edges.1.id: Duplicate edge id …. The consumer suites below are the same reading over every test-fixture flow that goes through the schema.

Pinned objectui tree (/home/user/objectui at a472b07, byte-equal to .objectui-sha, read-only): 89 edges[] arrays / 104 edges, 17 literal ids, 75 computed (React-Flow state, not authored flows), 0 duplicate literal ids; the computed 75 are NOT statically decidable. hotcrm: NOT MEASURED (not checked out here; its one instance was renumbered in hotcrm PR #1571 per the card).

Verdict: zero in-repo instances remain, so the changeset carries the release-note remedy (renumber) and names no file to migrate.

Reverse verification (ablation, trap-restored)

The fix committed first; flow.zod.ts put back to its BASE text (git restore --source=a55efc6c1) under an EXIT INT TERM trap that restores from HEAD by absolute path. Mutation proven on disk before running: marker Duplicate edge id count 1 → 0, superRefine count 3 → 2, tree blob abc5dc7 ≠ HEAD blob a0d5cf2. Expected direction: the four refusal pins turn red, the control and unique-id pins stay green. Observed: Tests 4 failed | 98 passed (102) — exactly those four (duplicate refused · rendered line · one-per-later-occurrence · defineFlow refuses). Restore: exit 0, git diff HEAD empty, git status --porcelain empty, tree blob = HEAD blob a0d5cf2, marker count back to 1. The test imports ./flow.zod from source, so no dist leg is on this path (no ablation-dist-preflight applies).

Consumer-package readings (prefix direction ...@objectstack/spec = downstream), at 260c4d986

All under scripts/pm/os-verify-lock.sh; each closure built first (turbo run build --filter="PKG^..." --concurrency=2), then pnpm --filter PKG exec vitest run --maxWorkers=2:

package reading verdict line
@objectstack/spec (build 133s, then whole suite) Test Files 472 passed · Tests 12,692 passed; typecheck (tsc + scripts + test-typecheck) OK VERDICT command-exit 0
@objectstack/lint Test Files 96 passed · Tests 3,006 passed, 5 skipped VERDICT command-exit 0
@objectstack/core Test Files 49 passed · Tests 1,190 passed VERDICT command-exit 0
@objectstack/service-automation Test Files 108 passed · Tests 1,294 passed VERDICT command-exit 0
example apps (app-crm, app-showcase, app-todo) every shipped flow parsed through the new schema, 0 refused (dynamic census above)

Declared narrowing: the example apps' own validate / test scripts were NOT run locally — their build closures are 59–63 packages each and would not fit the foreground cap on a shared box; their flows were parsed directly instead (above), and CI's Test Core / Type Check · consumer gates run the scripts.

Gates

Family derived with node scripts/pm/dispatch-gates.mjs (no paths) from the actual diff at a4da73eec (121 commands); the runnable ones executed, then the union re-run at the final commit 260c4d986. Six dist-reading gates (check:generated via check:api-surface, check:exported-any, check:dual-source-exports, check:entry-nameability, check:browser-reachable-entries, check:skill-examples) first answered STALE there — the ablation's restore rewrote flow.zod.ts with identical bytes but a newer mtime than the dist — which is their prerequisite refusal, not a reading; @objectstack/spec was rebuilt at 260c4d986 (same .build-input-hash 7c08850e…) and they were re-run: green (check:skill-examples additionally needs @objectstack/client-react built for its client-SDK surface; its reading is in the gate comment). The per-gate verdict lines from the re-run are in the gate comment on this PR. Green: check:generated (all 15 artifacts, incl. api-surface, authorable-surface, docs, strictness-ledger, liveness), check:empty-state, check:yaml-examples, check:error-code-provenance, check:exported-any, check:dual-source-exports, check:entry-nameability, check:llms-txt, check:variant-docs, check:browser-reachable-entries, lint's check:doc-formula-expressions (after building @objectstack/formula — its first run was PREREQUISITE NOT MET, not a finding), check:nul-bytes, check:cross-package-test-inputs, check:doc-authoring, check:spec-parsed-alias, check:test-source-alias, check:type-source-resolution, check:merge-driver, check:pm-half-states, check:objectui-changeset, check:published-files, check:org-identifier, check:page-declaration-shape, check:slot-lookup, check:where-matcher, check:objectql-double-limit, check:logger-receiver-detach, the changeset trio (check-empty-changeset, check-changeset-no-major, check-adr-0087-registration --base a55efc6c1 — "1 declared-breaking changeset(s), each carrying an ADR-0087 disposition") and check:changeset-gate-self-tests, plus the self-tests the derivation named (comment-mask, keyed-text-bounds, plugin-teardown, tenancy census, reference-carrier-shape, dev-prereqs, ci-filter-parity, system-context, registry-log, undeclared-dep-imports). NOT MEASURED locally: check:react-declaration-parity (external input — objectui's sdui.manifest.json, by design), check:published-readme-exports (needs a full-repo build; reads READMEs this diff does not touch), and the CI-step spellings the derivation lists with RUNNER_TEMP / matrix variables (ESLint, workspace type-check, shard attestations) — CI owns those.

Out of scope

nodes[].id uniqueness is not this card: measured equally unenforced on origin/main (two nodes with id: 'n' parse; the same schema refuses an invalid node type), filed as #15713 (unassigned, finding label; two lit controls — a node missing its required label, an unknown node key — because the node-type vocabulary is open under ADR-0018 and an invalid type cannot serve as one). #15713 remains open and is not addressed here. Related open cards not touched: #15617 (FlowRunSummary, execution.zod.ts), #14945 (end node outcome: 'refused', writes this same file after this PR lands), #13681, #14500.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf


Generated by Claude Code

…e id twice (#14964)

A superRefine on the flow's edges[] refuses a duplicate edge id at parse
time with an issue naming the id and both positions, anchored on the later
edge so the formatted error points at the one to renumber. Pins: the
duplicate refused, the card's control (invalid edge type) still refused, a
unique-id flow accepted, the rendered message shape.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf
…d at parse, ADR-0087 no-migration-prescription

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf
…, not a migration prescription

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 1 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via FlowSchema (symbol, a top-level const))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via FlowSchema (symbol, a top-level const))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 129 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8e8860ed8144feb6e5151c5a95655cb5bcb40f1epackageMentionDocs.

Which tree this was computed on

This run read content/docs from 593d9de040afa065a7bd7dec5f4a74ceb6717a57 — the merge of head 260c4d9864b34179d37135a8cb7bd6b964db8a25 into base 8e8860ed8144feb6e5151c5a95655cb5bcb40f1e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 593d9de040afa065a7bd7dec5f4a74ceb6717a57 && git checkout 593d9de040afa065a7bd7dec5f4a74ceb6717a57
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8e8860ed8144feb6e5151c5a95655cb5bcb40f1e 260c4d9864b34179d37135a8cb7bd6b964db8a25 && git checkout -B drift-repro 8e8860ed8144feb6e5151c5a95655cb5bcb40f1e && git merge --no-ff 260c4d9864b34179d37135a8cb7bd6b964db8a25

node scripts/docs-audit/affected-docs.mjs --json 8e8860ed8144feb6e5151c5a95655cb5bcb40f1e

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 8e8860ed8144feb6e5151c5a95655cb5bcb40f1e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Gate readings for this PR — union re-run at the final commit 260c4d986 (git rev-parse --short HEAD, tree clean)

Every exit code below was captured before any pipe (cmd › file 2›&1; EXIT=$?), and the quoted line is the gate's own last verdict line. Six dist-reading gates first answered STALE at this sha because the reverse-verification restore rewrote flow.zod.ts with identical bytes but a newer mtime than dist — a prerequisite refusal, not a reading; @objectstack/spec was rebuilt (same .build-input-hash 7c08850e…, VERDICT command-exit 0) and those six were re-run, one of them after also building the @objectstack/client-react closure it reads.

exit gate verdict line
0 pnpm --filter @objectstack/spec check:generated — first STALE (dist older than src after the ablation restore); spec rebuilt at 260c4d9, re-run ✓ All 15 generated artifacts are up to date.
0 pnpm --filter @objectstack/spec run check:empty-state ✓ all classified (2 closed, 2 open, 4 output, 8 scope)
0 pnpm --filter @objectstack/spec run check:yaml-examples ↳ 18 component node(s) also judged against their ComponentPropsMap props schema; 1 skipped (no row for the type — SDUI blocks and custom.* are
0 pnpm --filter @objectstack/spec run check:skill-examples — first STALE (dist older than src after the ablation restore), rebuilt spec, then PREREQUISITE (client-react unbuilt), built that closure, re-run ✅ 257 prose examples type-check across 3 surface(s) — every marked block parsed, so tsc ran the SEMANTIC pass on all of them
0 pnpm --filter @objectstack/spec run check:error-code-provenance OK — every registered-code stamp site is listed under its own owner key or carries a recorded waiver (9 waiver(s), all live)
0 pnpm --filter @objectstack/spec run check:exported-any — first STALE (dist older than src after the ablation restore); spec rebuilt at 260c4d9, re-run ✅ no exported type resolves to any: 2457 types + 1521 schemas across 17 entry points.
0 pnpm --filter @objectstack/spec run check:dual-source-exports — first STALE (dist older than src after the ablation restore); spec rebuilt at 260c4d9, re-run ✅ no new dual-source exports: 5060 names across 17 entry points — 206 re-exported (single declaration), 0 accepted dual-source (baseline).
0 pnpm --filter @objectstack/spec run check:entry-nameability — first STALE (dist older than src after the ablation restore); spec rebuilt at 260c4d9, re-run ⚠️ NOT MEASURED: no callable export on @objectstack/spec/qa.
0 pnpm --filter @objectstack/spec run check:llms-txt ✓ packages/spec/llms.txt: 97 claim(s) re-derived — every advertised symbol resolves against api-surface/ (17 entry points), every subpath is a rea
0 pnpm --filter @objectstack/spec run check:variant-docs ✓ variant/doc gate: 18 discriminated union(s) — 8 governed (every variant mentioned in a bound doc), 10 exempt.
0 pnpm --filter @objectstack/spec run check:browser-reachable-entries — first STALE (dist older than src after the ablation restore); spec rebuilt at 260c4d9, re-run ✅ check:browser-reachable-entries — 2 declared browser-reachable entries link no zod; 44 bundle(s) scanned, 40 zod link(s) seen elsewhere (instru
0 pnpm --filter @objectstack/lint run check:doc-formula-expressions #11673).
0 pnpm check:nul-bytes check-nul-bytes: OK (scanned 7584 text file(s) -- 7584 tracked, 0 untracked-not-ignored; skipped 7 binary; no raw ASCII control bytes).
0 pnpm check:cross-package-test-inputs OK: 27 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
0 pnpm check:doc-authoring ✓ doc authoring guard: sibling-package prose ids hold the baseline — 831 pinned site(s) across 231 file(s), 85085 string(s) read in 1153 parsed so
0 pnpm check:docs-audit-scope ✓ scope injection is live: the workflow audits the list handed in as args.handwritten, and refuses an invocation that hands in no scope at all.
0 pnpm check:changeset-gate-self-tests ✓ check-changeset-no-major --self-test: 116 assertions (frontmatter dialects measured against @changesets/parse + the pre/exit exemption switch in b
0 node scripts/check-empty-changeset.mjs --base a55efc6c1b1d8220e523c690d92652d0dd3e3c13 ✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
0 node scripts/check-changeset-no-major.mjs --base a55efc6c1b1d8220e523c690d92652d0dd3e3c13 ✓ This diff introduces no major bump.
0 node scripts/check-adr-0087-registration.mjs --base a55efc6c1b1d8220e523c690d92652d0dd3e3c13 ::notice file=.changeset/flow-edge-id-uniqueness.md::ADR-0087 exemption (no-migration-prescription): No authorable key is renamed, retired or re-typed
0 pnpm check:pm-half-states ✓ check-half-states self-test: 2172 cases pass.
0 pnpm check:objectui-changeset ✓ objectui-range --self-test: all checks passed
0 node scripts/check-keyed-text-bounds.mjs ⚠ The delta is information, not a verdict — this population grows AND shrinks for good reasons, and only the floors decide. Reproduce the record
0 pnpm check:spec-parsed-alias ADR-0122 type-alias convention: 1516 bare z.input aliases, 825 pinned isomorphic, 691 paired with an XParsed. OK
0 pnpm check:test-source-alias check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through dist/; 48 published subpath(
0 pnpm check:merge-driver ✓ check-regen-pending self-test passed.
0 node packages/lint/scripts/check-reference-carrier-shape.mjs position unresolved: 36 site(s) + 12 conflicting — 0 material (a refusal here is scoped to a non-string literal, where the readings disagr

Family derivation (node scripts/pm/dispatch-gates.mjs, no paths, tree a4da73eec — same spec/test bytes as 260c4d986; only the changeset text differs): 121 commands, 94 exit 0. The 27 non-zero: check:doc-formula-expressions was PREREQUISITE NOT MET (formula unbuilt; built, then green — row above), check:react-declaration-parity is the documented external-input gate (objectui sdui.manifest.json; exits 1 by design here), check:published-readme-exports needs a full-repo build (reads READMEs this diff does not touch — NOT MEASURED locally), and the remaining 24 are CI-step spellings the derivation lists with RUNNER_TEMP / matrix variables or job labels (ESLint, workspace type-check, shard attestations, claim-path and label steps) — CI owns those.

Consumer suites (scripts/pm/os-verify-lock.sh, prefix direction ...@objectstack/spec = downstream; closures built first with turbo run build --filter="PKG^..." --concurrency=2):

  • @objectstack/spec — Test Files 472 passed · Tests 12692 passed · typecheck OK — VERDICT command-exit 0 · held the lock 342s
  • @objectstack/lint — Test Files 96 passed · Tests 3006 passed, 5 skipped; @objectstack/core — Test Files 49 passed · Tests 1190 passed — one hold, VERDICT command-exit 0 · held the lock 257s
  • @objectstack/service-automation — Test Files 108 passed · Tests 1294 passed — VERDICT command-exit 0 · held the lock 183s
  • example apps — every shipped flow parsed through the built FlowSchema (dist/automation): 50 distinct flow exports (73 rows counting re-exports) across 7 files, 0 refused; control refused at edges.1.id. Their validate/test scripts NOT run locally (59–63-package closures) — declared narrowing, CI runs them.

Reverse verificationflow.zod.ts at its BASE text under an EXIT/INT/TERM trap: marker 1 → 0, blob abc5dc7 ≠ HEAD a0d5cf2; Tests 4 failed | 98 passed (102) — exactly the four refusal pins; restore exit 0, git diff HEAD empty, blob back to a0d5cf2.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 5, 2026 06:44
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 52804cd Sep 5, 2026
43 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-14964-flow-edge-id-uniqueness branch September 5, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FlowSchema accepts a flow whose edges[] declares the same id twice — measured with a control, and it let a duplicate id ship on green CI

2 participants